-
Notifications
You must be signed in to change notification settings - Fork 135
add JooqBatchWithoutBindArgs check #2506
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Generate changelog in
|
3581cdc to
d36cbed
Compare
| private static final String ERROR_MESSAGE = | ||
| "Jooq batch methods that execute queries without bind args cause performance problems."; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if @lukaseder has guidance on this?
@berler It would be helpful if you could provide more information. How do we ensure this remains true over time? Do you have public benchmarking data to substantiate the claim?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't have public benchmarking data, but we do know that the way this executes is using extra CPU on the database because each query is sent over as a unique string (with all variable values inline as a string in the query). There is CPU overhead parsing and query planning each query in the batch.
Looking at how we we have used jooq internally, it was actually surprising to most of our dev teams how these calls were being executed. People usually use the batch methods to execute the same query hundreds to thousands of times but just with different bind variables. What we want to happen (and what most of our devs thought was actually happening) is it sends the query to the DB once and then just sends the arrays of bind args to execute the same query with each set of bind variables. Internally we've already refactored nearly all of our code to use the other batch methods that execute with bind args.
...ine-error-prone/src/main/java/com/palantir/baseline/errorprone/JooqBatchWithoutBindArgs.java
Outdated
Show resolved
Hide resolved
...ine-error-prone/src/main/java/com/palantir/baseline/errorprone/JooqBatchWithoutBindArgs.java
Outdated
Show resolved
Hide resolved
...ine-error-prone/src/main/java/com/palantir/baseline/errorprone/JooqBatchWithoutBindArgs.java
Show resolved
Hide resolved
CVdV-au
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ERROR please!
|
Released 4.191.0 |
###### _excavator_ is a bot for automating changes across repositories. Changes produced by the roomba/latest-baseline-oss check. # Release Notes ## 4.191.0 | Type | Description | Link | | ---- | ----------- | ---- | | Feature | Add error-prone check JooqBatchWithoutBindArgs | palantir/gradle-baseline#2506 | To enable or disable this check, please contact the maintainers of Excavator.
Before this PR
Jooq provides multiple methods to execute queries in batches. Some of these methods end up executing queries without bind args which can cause performance problems.
After this PR
Disallow jOOQ's batch methods that execute queries without bind args. Batch methods that do use bind args should be used instead.
==COMMIT_MSG==
Add error-prone check JooqBatchWithoutBindArgs
==COMMIT_MSG==
Possible downsides?